Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add AsyncioRunnable #411

Merged
merged 51 commits into from
Nov 2, 2023
Merged

Conversation

cwharris
Copy link
Contributor

Moves the CoroutineRunnable from Morpheus' Sherlock feature branch to MRC and renames it to AsyncioRunnable as it is heavily dependent on asyncio. Adjustments were made such that the Scheduler would no longer own a task container and/or tasks, leaving the scheduler interface simpler. Instead, the runnable is responsible for the lifetime of the tasks it creates. This leaves the scheduler with a single responsibility.

Much of the code could be moved to MRC proper from PyMRC, but it's not immediately obvious where the code should live or whether it would be reused, so keeping it colocated with the AsyncioRunnable makes the most sense for now, imo.

@cwharris cwharris requested review from a team as code owners October 30, 2023 18:34
@cwharris cwharris added non-breaking Non-breaking change feature request New feature or request labels Oct 30, 2023
@cwharris cwharris mentioned this pull request Oct 30, 2023
python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp Outdated Show resolved Hide resolved
python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp Outdated Show resolved Hide resolved
python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp Outdated Show resolved Hide resolved
python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp Outdated Show resolved Hide resolved
python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp Outdated Show resolved Hide resolved
python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp Outdated Show resolved Hide resolved
python/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp Outdated Show resolved Hide resolved
Copy link

codecov bot commented Nov 2, 2023

Codecov Report

Merging #411 (f2369db) into branch-23.11 (1ebd4e2) will increase coverage by 0.28%.
The diff coverage is 95.04%.

Additional details and impacted files

Impacted file tree graph

@@               Coverage Diff                @@
##           branch-23.11     #411      +/-   ##
================================================
+ Coverage         73.45%   73.73%   +0.28%     
================================================
  Files               392      395       +3     
  Lines             13862    14132     +270     
  Branches           1050     1078      +28     
================================================
+ Hits              10182    10420     +238     
- Misses             3680     3712      +32     
Flag Coverage Δ
cpp 70.24% <95.04%> (+0.93%) ⬆️
py 42.61% <25.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
cpp/mrc/include/mrc/coroutines/scheduler.hpp 100.00% <ø> (+100.00%) ⬆️
...pp/mrc/src/public/exceptions/exception_catcher.cpp 100.00% <100.00%> (ø)
python/mrc/_pymrc/include/pymrc/coro.hpp 94.11% <100.00%> (+6.81%) ⬆️
...hon/mrc/_pymrc/include/pymrc/asyncio_scheduler.hpp 88.23% <88.23%> (ø)
...thon/mrc/_pymrc/include/pymrc/asyncio_runnable.hpp 95.31% <95.31%> (ø)

... and 14 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1ebd4e2...f2369db. Read the comment docs.

@cwharris
Copy link
Contributor Author

cwharris commented Nov 2, 2023

/merge

@rapids-bot rapids-bot bot merged commit 62e1834 into nv-morpheus:branch-23.11 Nov 2, 2023
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request non-breaking Non-breaking change
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants